Skip to content

fix: retain companion sessions and bound large history loading - #1685

Draft
Shopalooza1 wants to merge 1 commit into
openclaw:mainfrom
Shopalooza1:fix/companion-session-catalog-retention
Draft

fix: retain companion sessions and bound large history loading#1685
Shopalooza1 wants to merge 1 commit into
openclaw:mainfrom
Shopalooza1:fix/companion-session-catalog-retention

Conversation

@Shopalooza1

@Shopalooza1 Shopalooza1 commented Oct 9, 2026

Copy link
Copy Markdown

What Problem This Solves

Sessions can disappear after an incomplete reconnect response, and large history reconstruction can stall the companion or discard already loaded entries.

User Impact

Adds bounded, cancellable history acquisition/projection, catalog retention and visible retry state, durable checkpoint helpers, and opt-in local hang diagnostics. Existing history and raw message identity are retained. Native freeze prevention is not yet proven.

Why This Change Was Made

History work is bounded across fetch, reconstruction and display. Catalog replacement requires a qualified response. Checkpoint persistence and hang collection have explicit failure and lifetime handling.

This is a consolidated snapshot of the existing independently reviewed four-workstream branch. Original incremental source HEAD: c0c235b05c8c4fce2d81d430102c7039f7c59cfe. API publication HEAD: a79b8c4410b59faf9fedc123365da4236cb3d7e5. Both have the exact Git tree 2591b2a34cd8dfb15457edd0cd36805ce7dc4182. Original history remains preserved in the owner handoff.

Evidence

Mini independently verified all 86 published blobs and exact source-tree equality. Current-source focused portable Tray tests: 55 passed, 0 failed, exit 0. Includes real synthetic socket/client/loader history replay, all 4,200 entries, cancellation/supersession, retained-state failures and distinct display IDs sharing a raw ID.
TRX SHA-256: F1EB3F17AF92726C2C3F8F6E736161DFD6E82A95B25B5C1DA522B8496FB37A3C.

Change Type

  • Bug fix
  • Feature
  • Tests or validation

Scope

  • Tray or WinUI UX
  • Gateway, connection, or pairing
  • Permissions, privacy, or security
  • Tests, CI, or docs

Required proof pools

  • windows-winui-interactive: history/retry UI, responsiveness and native hang capture.
  • windows-wsl-gateway-e2e: real reconnect/history protocol and consumer integration.
  • windows-11-sac-on: generated diagnostics binaries and trusted-signature runtime validation.
    These remain requested proof requirements, not evidence of execution or permission to provision hosts, spend, or schedule jobs. Maintainer approval and an existing authorized host are required.

Validation

  • Full isolated Windows build at the original source HEAD: exit 0, using Mini's private SDK/build adapter. Unmodified developer-host build remains to be confirmed.
  • Focused portable OpenClaw.Tray.Tests: 55 passed, 0 failed.
  • Required full Shared/Tray/Connection runtime suites: NOT VERIFIED at this final source snapshot. Recorded native attempts are blocked by enforcing Smart App Control/Code Integrity, including generated Connection.dll rejection 0xc0e90002.
  • Required closeout commands on an approved trusted Windows host:
    • ./build.ps1
    • dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore
    • dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore
    • dotnet test ./tests/OpenClaw.Connection.Tests/OpenClaw.Connection.Tests.csproj --no-restore
      Fresh projects must restore/build before --no-restore; zero-test success is not acceptance.

Real Behavior Proof

  • Environment tested: isolated portable .NET fixture lane and isolated Windows build lane.
  • Commit tested: original source c0c235b0; exact tree equality verified for publication a79b8c4.
  • Evidence after fix: 55 passing focused tests, including real synthetic socket/client/loader replay and structural merge identity controls.
  • Screenshot or artifact links verified? N/A. Local receipts retained by Mini; no private logs/history uploaded.
  • Not verified / blocked: interactive WinUI, full real Gateway path, native UI-thread stack/unwind capture, complete runtime suites and freeze prevention.
  • Independent review: Mini's source-grounded review and counterexample-driven iterations retained in the owner handoff. No additional paid autoreview dispatch performed.

Security Impact

  • New permissions or capabilities? No new external permissions; local opt-in hang-capture executable added.
  • Secrets or tokens handling changed? Yes. Existing DPAPI credential read retries only sharing/lock violations, bounded to 500 ms; no secret rotation or credential contents published.
  • New or changed network calls? Existing Gateway paging/catalog requests change; no new external endpoint.
  • Command or tool execution surface changed? Yes, local diagnostics collector/lifetime handling. Native validation remains required.
  • Data access scope changed? Local diagnostic capture must remain opt-in and redacted; no private migration state included.

Compatibility and Migration

Existing installed companion and gateway remain unchanged. No deployment, activation, merge, source deletion or trust-policy changes are included. Protocol/page compatibility and native diagnostics require the proof above.

Review Conversations

  • I replied to or resolved every bot review conversation addressed by this PR.
  • I left unresolved only conversations that still need maintainer judgment.

@Shopalooza1
fix: retain companion sessions and bound history reconstruction
a79b8c4 
Publish the independently reviewed OpenClaw reliability source snapshot.
Source commit: c0c235b05c8c4fce2d81d430102c7039f7c59cfe
Exact source tree: 2591b2a

Includes bounded history projection, catalog recovery, resumable checkpoints,
and local hang-capture diagnostics. Windows native UI and full required
runtime suites remain blocked by Code Integrity; no freeze prevention or
production acceptance is claimed.

Original incremental source history and validation receipts remain preserved
in the existing Book-to-Mini handoff. This API publication is a snapshot
commit with identical source content, not a rewrite of that history.
@clawsweeper

clawsweeper Bot commented Oct 9, 2026 •
edited
Loading

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P1 Urgent regression or broken agent/channel workflow affecting real users now. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Oct 9, 2026
@clawsweeper

clawsweeper Bot commented Oct 9, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge.

What this changes

This PR adds session retention, bounded history loading and navigation, durable checkpoint helpers, and local hang diagnostics.

Example: A reconnect returns an empty session list without confirmed inventory metadata.

  • Before: Previously listed conversations disappear.
  • After: The conversations remain listed and the Sessions page offers Retry.

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The consumer cutover and retained-state defects block correctness, and the expanded product scope remains undecided.
Proof confidence 🧂 unranked krab (1/6) Real behavior proof is necessary before merge. See Before merge.
Patch quality 🧂 unranked krab (1/6) 4 actionable review findings remain.

Product

Kind: Bug fix · Worth it: Needs a maintainer decision · Fix scope: Partial
User problem: Conversations can disappear after incomplete reconnect responses, and restoring history can leave native chat unresponsive or lose already loaded entries.
Reason: Retention and responsiveness have clear user value, but the new checkpoint storage and diagnostic CLI broaden the product surface without recorded owner direction.

Merge readiness

⛔ Blocked before merge - 10 items remain

This PR contains distinct reliability work still absent from main, but its history cutover and retained-state handling need correction before merge.

Priority: P1
Reviewed head: a79b8c4410b59faf9fedc123365da4236cb3d7e5
Owner decision: Required. See Decision needed.

Decision needed

  • Question: Should this reliability repair also introduce an unused durable checkpoint framework and standalone native diagnostic executable?
  • Recommendation: Separate the additional capabilities: Keep this PR focused on catalog and history correctness; review checkpoint integration and diagnostics with concrete consumers separately.
  • Why: Neither addition is needed by the current retention consumer, and no recorded owner approval establishes their inclusion in this repair.

Before merge

  • Add real behavior proof - Relevant supplied execution is portable socket/client/loader harness coverage. Provide current-head screenshots or a recording of native restoration, older/newer navigation and catalog Retry, plus redacted runtime output for supported Gateway compatibility and the diagnostic executable. Redact credentials, private endpoints, IPs, and conversation content. After adding proof, update the PR body, then request a new review with @clawsweeper re-review. The PR author or someone with repository write access can request it.
  • Preserve compatibility with supported Gateway history responses (P1) - The captured producer fixture returns 3,045 messages without completeSnapshot or completeCliImport. This guard rejects that response for the production loader's limit of 200, and the byte guard also rejects its approximately 6.29 MB message array. The added Mini_ActualHandlerStripsLossMetadata_ValidPartialBeyondRequestIsRejected test explicitly records the incompatibility. Since every initial load now uses this carrier, existing conversations fail to load. Preserve a bounded compatible path or complete and verify the producer cutover before making this consumer mandatory.
  • Withhold replacement authority when content was omitted (P1) - A same-session forced refresh with HasMore=false and OmittedCount>0 still becomes authoritative here. MergeWithLiveEntries then drops unmatched retained entries whose sequence is within the rebuilt range, despite the response reporting missing content. The omission test starts with an empty timeline and misses this loss. Require content-completeness evidence before pruning and cover a populated timeline followed by an omission-bearing terminal response.
  • Keep retained older entries before a refreshed history tail (P2) - For an incomplete same-session refresh, this call initializes the merged timeline with the recent tail and then appends unmatched prior entries. The added BoundedIncompleteTail_PreservesPreviouslySeenOlderEntries case therefore places tail at sequence 9 before older at sequence 1; it checks membership without checking order. Reconnecting after loading older history can reorder the conversation and make an old message appear last. Merge in source chronology and assert the resulting order.
  • Keep mutable history state with the existing runtime owners (P2) - This adds a private history-state lock and mutable operation collection to the provider facade, contrary to the chat-provider-state-closed architecture row and the root AGENTS.md prohibition on restoring closed responsibilities. The loader already owns fetch lifetime and single-flight admission, while conversation state owns accepted history identity. Move this bookkeeping into those existing owners and keep the provider forwarding typed state and results. This concern was equally visible at the earlier reviewed head and was missed in that review.
  • Resolve merge risk - The contributor must resolve the GitHub-reported merge conflict before landing.
  • Complete next step - Run the unmodified full build and complete Shared, Tray, and Connection test suites on a trusted Windows host after repairs, confirming nonzero test execution.
  • Resolve maintainer decision - Resolve the maintainer decision shown above before merge.
  • Add data-model compatibility proof - The review found that existing stored data may not work after upgrade. Show that existing data still loads and works with this change.
  • Explain or restore the original intent of History lifetime and atomic state ownership - refactor(chat): extract runtime state owners #1096: Separate fetch lifetime from atomic conversation commits and prevent mutable runtime state from returning to the provider facade.

Findings

  • [P1] Preserve compatibility with supported Gateway history responses — src/OpenClaw.Shared/OpenClawGatewayClient.cs:739-741
  • [P1] Withhold replacement authority when content was omitted — src/OpenClaw.Tray.WinUI/Chat/ChatConversationState.cs:600-601
  • [P2] Keep retained older entries before a refreshed history tail — src/OpenClaw.Tray.WinUI/Chat/ChatConversationState.cs:628-633

Provenance

  • History lifetime and atomic state ownership changes intended behavior without addressing why it exists (refactor(chat): extract runtime state owners #1096: Separate fetch lifetime from atomic conversation commits and prevent mutable runtime state from returning to the provider facade.)

Tests

  • Low-value test tests/OpenClaw.Shared.Tests/HistoryPagingBudgetTests.cs: Remove these helper-arithmetic and incidental-budget assertions in favor of acquisition and cancellation behavior coverage.
  • Missing end-to-end proof: Show current-head native restoration, older/newer navigation and catalog Retry against a supported Gateway, including a latest-stable upgrade; the supplied tests do not establish the required base-failure/head-pass native scenario.
Agent review details

How this fits together

Companion receives session catalogs and conversation history over the Gateway WebSocket, reconstructs retained conversation state, and projects it into native WinUI chat.

flowchart TD
  A[Gateway WebSocket] --> B[Session catalog admission]
  A --> C[History page admission]
  B --> D[Sessions and Retry]
  C --> E[History loader]
  E --> F[Conversation state]
  F --> G[Native history window]
Loading

Technical review

Best possible solution:

Land a focused, backward-compatible retention repair through the existing history owners, preserving received content and chronology while separating unsponsored checkpoint and diagnostic capabilities.

Do we have a high-confidence way to reproduce the issue?

The captured producer fixture and retained-history merge code establish concrete failing inputs without execution; the sustained native UI hang has not been reproduced on current main by this review.

Is this the best way to solve the issue?

Bounding acquisition and projection is useful, but the consumer must preserve supported Gateway responses and retained ordering; related dump evidence also requires distinguishing reconstruction work from native tail-scroll hangs.

Full review comments:

  • [P1] Preserve compatibility with supported Gateway history responses — src/OpenClaw.Shared/OpenClawGatewayClient.cs:739-741
    The captured producer fixture returns 3,045 messages without completeSnapshot or completeCliImport. This guard rejects that response for the production loader's limit of 200, and the byte guard also rejects its approximately 6.29 MB message array. The added Mini_ActualHandlerStripsLossMetadata_ValidPartialBeyondRequestIsRejected test explicitly records the incompatibility. Since every initial load now uses this carrier, existing conversations fail to load. Preserve a bounded compatible path or complete and verify the producer cutover before making this consumer mandatory.
    Confidence: 0.99
  • [P1] Withhold replacement authority when content was omitted — src/OpenClaw.Tray.WinUI/Chat/ChatConversationState.cs:600-601
    A same-session forced refresh with HasMore=false and OmittedCount>0 still becomes authoritative here. MergeWithLiveEntries then drops unmatched retained entries whose sequence is within the rebuilt range, despite the response reporting missing content. The omission test starts with an empty timeline and misses this loss. Require content-completeness evidence before pruning and cover a populated timeline followed by an omission-bearing terminal response.
    Confidence: 0.99
  • [P2] Keep retained older entries before a refreshed history tail — src/OpenClaw.Tray.WinUI/Chat/ChatConversationState.cs:628-633
    For an incomplete same-session refresh, this call initializes the merged timeline with the recent tail and then appends unmatched prior entries. The added BoundedIncompleteTail_PreservesPreviouslySeenOlderEntries case therefore places tail at sequence 9 before older at sequence 1; it checks membership without checking order. Reconnecting after loading older history can reorder the conversation and make an old message appear last. Merge in source chronology and assert the resulting order.
    Confidence: 0.99
  • [P2] Keep mutable history state with the existing runtime owners — src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs:740-741
    This adds a private history-state lock and mutable operation collection to the provider facade, contrary to the chat-provider-state-closed architecture row and the root AGENTS.md prohibition on restoring closed responsibilities. The loader already owns fetch lifetime and single-flight admission, while conversation state owns accepted history identity. Move this bookkeeping into those existing owners and keep the provider forwarding typed state and results. This concern was equally visible at the earlier reviewed head and was missed in that review.
    Confidence: 0.98
    Late finding: first raised on code an earlier review cycle already covered.

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against bff8c0bc71a3.

Merge-risk options

Maintainer options:

  1. Resolve the reported conflict (recommended)
    The contributor resolves the dirty merge while preserving current-main behavior and the focused repair.
  2. Pause the draft
    Keep the draft parked until its author can reconcile the conflicting changes.

Provenance checked

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

  • Verified introduced change: The pinned merge-base-to-head diff contains the retention, paging, checkpoint, and diagnostics additions; HEAD remains the previously reviewed revision. (a79b8c4410b5)
  • Main still lacks the central repair: Current main requests history through the existing unpaged API and removes catalog keys absent from a response without the proposed completeness qualification. (src/OpenClaw.Shared/OpenClawGatewayClient.cs:4667, bff8c0bc71a3)
  • Captured producer incompatibility: The added actual-producer test expects rejection of a legitimate legacy complete-CLI response; its passing counterpart requires corrected producer metadata. Every initial production load now uses the strict carrier with limit 200 and maxBytes 524288. (tests/OpenClaw.Shared.Tests/ChatHistoryPageCarrierTests.cs:50, a79b8c4410b5)
  • Authoritative dependency contract: The patch explicitly tests against materialized Gateway producer output, making that protocol an applicable dependency contract. The inspected upstream handler still removes omission metadata before constructing response fields, and its response-page source supplies no completeCliImport flag. Dependency policy was read for this protocol boundary only. (src/gateway/server-methods/chat-history-handler.ts:304, 61deaaef5879)
  • Retained-history defects: CommitHistory grants replacement authority from HasMore alone, ignoring reported loss, and feeds an incomplete refreshed tail into a merge that appends unmatched older entries. (src/OpenClaw.Tray.WinUI/Chat/ChatConversationState.cs:600, a79b8c4410b5)
  • Architecture closure: The chat-provider-state-closed ledger forbids a private state lock and mutable history collections in the provider; the patch adds _loadOlderGate and _loadOlder there. The existing guard checks older field spellings and does not catch these additions. (docs/ARCHITECTURE.md:796, a79b8c4410b5)

Likely related people:

  • Christine Yan: Raw commit 8c9acbc adds src/OpenClaw.Tray.WinUI/Services/DiagnosticsBundleBuilder.cs:53 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: medium; commits: 8c9acbcfb5d8; files: src/OpenClaw.Tray.WinUI/Services/DiagnosticsBundleBuilder.cs)

Review metrics

Metric Value Why it matters
Production and test growth Production +7,132/-237 lines; tests +7,750/-22 lines The change combines the retention repair with substantial new storage, navigation, and diagnostic machinery; the broader scope needs an explicit owner decision.

Labels

Label changes:

No label changes.

Label justifications:

  • P1: The mandatory history cutover can break existing conversation restoration.
  • merge-risk: 🚨 compatibility: Strict page admission rejects a captured response supported by the existing Gateway contract.
  • merge-risk: 🚨 session-state: Refresh commits can discard retained entries or place older messages after the refreshed tail.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

History

Review history (1 earlier review cycle)
  • reviewed 2026-10-09T01:58:39.004Z sha a79b8c4 :: needs real behavior proof before merge. :: [P1] Preserve compatibility with current Gateway history responses | [P1] Withhold replacement authority when a page reports omitted content | [P2] Merge retained older entries before a refreshed history tail

Reviewed October 24, 2026, 5:39 PM ET / 21:39 UTC (Revision 2).

This was referenced Oct 13, 2026
Open
Closed
@github-actions

github-actions Bot commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

This pull request has been automatically marked as stale due to inactivity.
Please update it or it will be closed.

@github-actions github-actions Bot added the stale label Oct 23, 2026
@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 24, 2026
@github-actions github-actions Bot removed the stale label Oct 25, 2026
@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

This pull request has been automatically marked as stale due to inactivity.
Please update it or it will be closed.

@github-actions github-actions Bot added the stale label Nov 9, 2026
@roboclaw-bot

roboclaw-bot commented Nov 10, 2026

Copy link
Copy Markdown
Contributor

PR cleanup review

Reviewed head: a79b8c4410b59faf9fedc123365da4236cb3d7e5. Main comparison: e59374c064f43d68f5c72d070d4c89ef2d20ec3b.

Thanks for preserving the detailed reliability work and documenting the proof gaps. Current main now has older-history paging and recovery from #2093 (improve(chat): align native conversation and tool activity with web), and #1732 (fix: prevent intermittent credential reads and false handshake timeouts) addresses the credential publication race. Those overlap parts of this draft, but the catalog-completeness retention and bounded reconstruction/windowing changes are not fully covered, so I would not close it as wholly superseded. At a79b8c4, the captured legacy history response is still rejected by the mandatory carrier, terminal omission can prune retained entries, and a partial tail refresh can place older messages last. Could this be narrowed onto current main using its existing history owners, with populated-history/order/compatibility regressions, while checkpoint storage and native diagnostics are decided separately? Full Windows suites and current-head native proof remain needed.

Action in this cleanup pass: leaving this PR open. No branch changes or runtime-proof waiver. The findings above come from source, discussion and recorded-CI review; Windows validation was not rerun in this pass.

@github-actions github-actions Bot removed the stale label Nov 11, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Shopalooza1 @roboclaw-bot